Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add doctor commands to interrogate levelDB queues #18732

Closed
wants to merge 24 commits into from
Closed

Add doctor commands to interrogate levelDB queues #18732

wants to merge 24 commits into from

Conversation

zeripath
Copy link
Contributor

Whilst looking at #17204 it appears that the unique-queues can end up in a weird corrupt state - I'm not certain how this happens - however the most difficult thing has been that there was no way to detect this.

In this PR I propose two new doctor commands:

  1. uniquequeues-corrupt which will look for queues where the internal set length and the internal queue length differ.
  2. queues-listdb which will simply list all the entries in the db.

The PR also adds a lot of additional tracing logs.

@zeripath
Ensure that conflict status is reset if there are no conflicts
b06b088 
The ConflictedFiles status should always be reset if there are no conflicts
this prevents conflicted files being left over.

Fix go-gitea#17204

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Add some more trace and debug logging to process and testpatch
db1931c 
Given the repeated issues here I've just added some plain trace logging
which should allow for easier tracing of when processes are created.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Add doctor commands for looking at the level DB
bd066ca 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added type/enhancement An improvement of existing functionality backport/v1.16 labels Feb 11, 2022
@zeripath zeripath added this to the 1.17.0 milestone Feb 11, 2022
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 12, 2022
zeripath
zeripath commented Feb 12, 2022
View reviewed changes
modules/process/manager.go Outdated Show resolved Hide resolved
Gusted
Gusted reviewed Feb 12, 2022
View reviewed changes
modules/doctor/queue.go Outdated Show resolved Hide resolved
modules/doctor/queue.go Show resolved Hide resolved
modules/doctor/queue.go Outdated Show resolved Hide resolved
modules/doctor/queue.go Show resolved Hide resolved
modules/doctor/queue.go Show resolved Hide resolved
modules/doctor/queue.go Show resolved Hide resolved
@zeripath
refix process manager trace ended
93d8275 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
No revive - we want this to stutter
53232c4 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@singuliere
Copy link
Contributor

It would be useful to verify the autofix logic actually works by adding a test that runs it twice and asserts it does not complain the second time around. Just a suggestion.

@lunny
Copy link
Member

lunny commented Jun 4, 2022

Please resolve the conflicts.

@zeripath
Merge remote-tracking branch 'origin/main' into reset-conflict-status
4e02994 
Unfortunately we need to remove the trace logging from the process
manager.
@zeripath
remove trace logging from process manager
a639e44 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@codecov-commenter

This comment was marked as duplicate.

Sign in to view
@lunny lunny modified the milestones: 1.17.0, 1.18.0 Jun 16, 2022
@zeripath zeripath mentioned this pull request Jul 12, 2022
Closed
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 12, 2022
@lunny lunny modified the milestones: 1.18.0, 1.19.0 Oct 17, 2022
tdesveaux added a commit to dontnod/fork-gitea that referenced this pull request Oct 18, 2022
@tdesveaux
Cherry-pick tracelog additions from go-gitea#18732
dd810d4
tdesveaux added a commit to dontnod/fork-gitea that referenced this pull request Oct 18, 2022
@tdesveaux
Cherry-pick tracelog additions from go-gitea#18732
21cdd74
tdesveaux added a commit to dontnod/fork-gitea that referenced this pull request Nov 4, 2022
@tdesveaux
Cherry-pick tracelog additions from go-gitea#18732
c2715ff
@yardenshoham yardenshoham added outdated/backport/v1.19 This PR should be backported to Gitea 1.19 and removed backport/v1.16 labels Feb 18, 2023
@yardenshoham yardenshoham modified the milestones: 1.19.0, 1.20.0 Feb 22, 2023
@zeripath zeripath mentioned this pull request Feb 26, 2023
Merged
@zeripath
Merge remote-tracking branch 'origin/main' into reset-conflict-status
9e8f61b
@zeripath
remove mismerged files
900bb10 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
add actions job queue name
99c2b4b 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
restore licenses deleted by mistake
71e16d6 
Signed-off-by: Andrew Thornton <art27@cantab.net>
@wxiaoguang
Copy link
Contributor

I am 99.99% sure that it doesn't need " to interrogate levelDB queues" after the queue rewriting.

And poor users could just clear their queue on the admin page, then everything becomes right again.

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label May 11, 2023
@wxiaoguang wxiaoguang removed this from the 1.20.0 milestone May 22, 2023
@wxiaoguang wxiaoguang removed the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label May 22, 2023
@wxiaoguang
Copy link
Contributor

wxiaoguang commented May 24, 2023 •
edited
Loading

-> Help to recover from corrupted levelqueue #24912

gitea.com experienced the corrupted LevelQueue bug again. I think the problem is clear now.

This PR's approach "checkUniqueQueues" doesn't work because the LevelQueue (LPop) stops working. The newly introduced "Remove All" button on admin page doesn't work either because it also uses LPop.

To "fix" the corrupted queue, the simplest way it is to just remove all keys with the queue prefixes, RemoveLevelQueueKeys in #24912

To "interrogate" the levelqueue, there is ListLevelQueueKeys in #24912 (the same as this PR's queueListDB)

@wxiaoguang
Copy link
Contributor

The newly written "logger system" and "queue system" seem stable, after 1.20 gets released, there is no bug report for them yet (till now). Not like before: untestable functions, fragile tests, various bug patches, unclear failures, etc.

So, what you worried about "You're just going to create more bugs and reiterate errors (23753)", I have confidence to say: it won't happen.

While I can't guarantee my code is 100% bug-free, so if there is any related bug, I will fix them in first time (as always).

@wxiaoguang wxiaoguang closed this Jul 22, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Gusted Gusted Gusted left review comments

@lunny lunny lunny left review comments

@6543 6543 6543 approved these changes

Assignees
No one assigned
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 1 This PR needs approval from one additional maintainer to be merged. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@zeripath @singuliere @lunny @codecov-commenter @wxiaoguang @6543 @Gusted @yardenshoham @GiteaBot